-
Notifications
You must be signed in to change notification settings - Fork 264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unexpected Balances #63
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vulnerabilities/force-feeding.md
Outdated
|
||
### Normal Contract Behavior | ||
|
||
In typical smart contract operation, Ether is sent to a contract via a transaction that calls a function or invokes the `receive()` or `fallback()` functions. If a contract lacks these functions, transactions sending Ether to it will normally be reverted, ensuring the contract does not inadvertently receive funds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth also noting that ETH can be sent along with payable functions
vulnerabilities/force-feeding.md
Outdated
@@ -0,0 +1,89 @@ | |||
# Force Feeding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a better title would be "Unexpected Ether Balance"? This way it focuses more on the vulnerability rather than the attack vector. Would have to update the below paragraph to reflect this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about "Unexpected Ether Transfer"?
vulnerabilities/hash-collision.md
Outdated
@@ -0,0 +1,109 @@ | |||
In Solidity, the `abi.encodePacked()` function is used to create tightly packed byte arrays which can then be hashed using `keccak256()`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As noted by rakesh, this got accidentally included it seems
## Preventing Force Feeding | ||
|
||
To safeguard against force-feeding, consider the following strategies: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self to come back to reconsider these prevention methods further
Actually, I think this fits best under https://github.com/kadenzipfel/smart-contract-vulnerabilities/blob/master/vulnerabilities/dos-revert.md#unexpected-balance. Can we instead provide a simplified version of this with the above recommended changes to the dos-revert file? |
@kadenzipfel the file at |
Yeah that's a good point. I think the reason I initially removed "forcibly sending ether" was because it was documented as an attack rather than as a vulnerability, but if we're documenting this instead as unexpectedly receiving ether then that's actually good to have imo. One thing I wonder is whether this should more generally encompass unexpected balances as a whole since the same type of issue could be caused by having an unexpected amount of tokens, e.g. an inflation attack. Then maybe would be ideal to include the ability to forcefully send ether as an odd evm gotcha to be aware of. Wdyt? |
That sounds good. Just to make sure I'm getting you correctly, the focus will be: Am I getting you right? |
Yep! |
README.md
Outdated
@@ -6,6 +6,7 @@ | |||
- [Timestamp Dependence](./vulnerabilities/timestamp-dependence.md) | |||
- [Authorization Through tx.origin](./vulnerabilities/authorization-txorigin.md) | |||
- [Floating Pragma](./vulnerabilities/floating-pragma.md) | |||
- [Force Feeding](./vulnerabilities/force-feeding.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should update the name here, also would be good to change the filename as well
unexpected-balances.md is still a work in progress and not yet ready for a merge. When done, it will supersede force-feeding as discussed in this [PR](kadenzipfel#63)
Related Issue
Checklist
Describe the changes you've made:
Added force-feeding as a technique used by attackers to send Ether directly to a smart contract address without invoking any of its functions. This can disrupt the contract's internal accounting mechanisms and logic, particularly if the contract relies on the assumption that its balance can only change through controlled transactions.
Type of change
Select the appropriate checkbox: